-
Notifications
You must be signed in to change notification settings - Fork 81
Filer - Receiver/Liquidator DRS publishing #4035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
queue_services/business-filer/src/business_filer/services/publish_event.py
Show resolved
Hide resolved
|
Can you update |
doug-lovett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, and I know it works. Just the one comment on using filings.id or filings.transaction_id to identify the filing.
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
I added pytest-cov too for the new CI changes. The test failures are happening off of main and are intermittent across several tests. Have you run into this before? It looks like there might be a runtime issue with the cascade deletions in the tests (seems to only come up in cases where addresses are being removed) |
don't think i come across this issue |
| "gunicorn (>=23.0.0,<24.0.0)", | ||
| "business-registry-account @ git+https://github.com/bcgov/lear.git@main#subdirectory=python/common/business-registry-account" | ||
| "business-registry-account @ git+https://github.com/bcgov/lear.git@main#subdirectory=python/common/business-registry-account", | ||
| "pytest-cov (>=7.0.0,<8.0.0)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move this to dev dependencies
| # FUTURE: DRS integration with document id No newline at end of file | ||
| try: | ||
| # Create DRS record | ||
| PublishEvent.publish_drs_create_message(current_app, business, filing_rec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its better to move this after commit, that's what we follow in filer for other messages https://github.com/bcgov/lear/blob/main/queue_services/business-filer/src/business_filer/services/filer.py#L277
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that makes sense - will do
this PR to the model fixes the issue (tested locally), but I'm not really sure why this started happening #4037 |
Issue #: /bcgov/entity#31316
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).